gRPC: server-side envelope propagation + correct false docs/sample claims (GH-3368) - #3369
Conversation
…le claims (JasperFxGH-3368) Wolverine.Grpc's client interceptor stamped correlation-id/tenant-id onto outgoing calls, but nothing on the server side read them back — despite docs and the OrderChainWithGrpc sample claiming a WolverineGrpcServicePropagationInterceptor already existed and did this. It didn't. Adds the real interceptor (registered by AddWolverineGrpc(), off via WolverineGrpcOptions.PropagateEnvelopeHeaders), fixes the false claims in docs/guide/grpc/client.md and the sample's XML comments, and covers the fix with end-to-end tests through a real Kestrel TestServer + Bus.InvokeAsync. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
This PR closes a gap in Wolverine.Grpc by adding server-side propagation of the correlation-id and tenant-id envelope headers from inbound gRPC metadata onto the ambient IMessageContext, and corrects documentation/sample text that previously claimed this behavior already existed.
Changes:
- Added
WolverineGrpcServicePropagationInterceptorto read inboundcorrelation-id/tenant-idmetadata and apply them toIMessageContext(behind a new option). - Added
WolverineGrpcOptions.PropagateEnvelopeHeaders(defaulttrue) and wired the interceptor intoAddWolverineGrpc()with correct interceptor ordering. - Added end-to-end tests covering propagation behavior and the disable switch; updated docs and sample comments to match actual behavior.
Reviewed changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| src/Wolverine.Grpc/WolverineGrpcServicePropagationInterceptor.cs | New server interceptor that copies inbound correlation-id/tenant-id metadata onto IMessageContext. |
| src/Wolverine.Grpc/WolverineGrpcOptions.cs | Adds server-side PropagateEnvelopeHeaders option (default true). |
| src/Wolverine.Grpc/WolverineGrpcExtensions.cs | Registers the new interceptor and ensures it runs inside (after) the exception interceptor. |
| src/Wolverine.Grpc.Tests/ServerPropagation/server_propagation_interceptor_tests.cs | End-to-end tests proving the full hop (client stamps → server reads → handler sees). |
| src/Wolverine.Grpc.Tests/ServerPropagation/server_propagation_disabled_tests.cs | Tests that server-side propagation can be disabled. |
| src/Wolverine.Grpc.Tests/ServerPropagation/PropagationEchoGrpcService.cs | Test gRPC service forwarding to Bus.InvokeAsync(...). |
| src/Wolverine.Grpc.Tests/ServerPropagation/PropagationEchoContracts.cs | Test gRPC contracts and DTOs for propagation scenarios. |
| src/Wolverine.Grpc.Tests/ServerPropagation/DisabledPropagationFixture.cs | In-process test host fixture with propagation disabled. |
| src/Wolverine.Grpc.Tests/add_wolverine_grpc_idempotency_tests.cs | Ensures interceptor registration idempotency and ordering. |
| src/Samples/OrderChainWithGrpc/InventoryServer/ReserveStockHandler.cs | Fixes sample XML comments to reflect the now-real interceptor and clarify trace propagation. |
| docs/guide/grpc/client.md | Corrects and expands docs to match actual server-side behavior and configuration. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
jeremydmiller
left a comment
There was a problem hiding this comment.
Reviewed — this is right, and the write-up is exemplary. Points that got specific scrutiny and passed:
- Unconditional set on header presence is the correct convention, and your reasoning is exactly why: a fresh
IMessageContext.CorrelationIdis pre-seeded (Activity root or GUID), so an "only if empty" guard would silently never fire. MatchingMessageContext.ReadEnvelopesemantics is the consistent choice. - Not propagating
parent-id/conversation-idis correct — no envelope exists at interceptor time, and trace lineage already rides W3C traceparent via Activity propagation. - Interceptor ordering (exception translation outermost, symmetric to the client side) and the scoped-
IMessageContextnull-bail are both right. - Correcting the docs/sample claims about a type that never existed: thank you for grepping instead of assuming.
Merging on green (24/24). Leaving #3368 open: this lands the propagation leg, and the remaining feature is the full ITenantDetectionPolicies mirror (claims-based + custom detectors + the codegen tenant-id variable so Marten/Polecat session-opening frames get the tenant without touching the ambient context). That work will build on this interceptor rather than replace it.
Summary
Wolverine.Grpc's client interceptor already stampedcorrelation-id/tenant-idonto outgoing calls, but nothing on the server side read them back — despitedocs/guide/grpc/client.mdand theOrderChainWithGrpcsample's XML comments claiming aWolverineGrpcServicePropagationInterceptoralready existed and did exactly this. It didn't; greppingsrc/Wolverine.GrpcconfirmedAddWolverineGrpc()only ever registered the exception interceptor.WolverineGrpcServicePropagationInterceptor(registered automatically byAddWolverineGrpc(), positioned inside the exception interceptor to mirror the client-side ordering). It readscorrelation-id/tenant-idoff inbound gRPC metadata and applies them to the ambientIMessageContextunconditionally when present — matching the convention every other Wolverine transport uses on a freshly received envelope (MessageContext.ReadEnvelope), rather than a "set only if empty" guard (a freshIMessageContext.CorrelationIdis never actually empty — it's pre-seeded fromActivity.Current— so that guard would have silently no-op'd, and an earlier draft of this change caught exactly that bug via its own tests).WolverineGrpcOptions.PropagateEnvelopeHeaders(defaulttrue) as the server-side off-switch, mirroring the existing client-side option.parent-id/conversation-idare deliberately not propagated this way — they live on anEnvelope, and there is no envelope yet at the point this interceptor runs (IMessageContext.Envelopeis null outside of message handling). Cross-process parent/trace correlation for gRPC hops already happens separately via OpenTelemetryActivitypropagation over HTTP/2.docs/guide/grpc/client.mdand theOrderChainWithGrpcsample'sReserveStockHandler.csXML comment now describe what's actually implemented instead of a type that never existed.Related to #3368 (the original Discord question was about tenant-id detection for external gRPC callers; this closes the Wolverine-to-Wolverine hop half of that gap and fixes the docs/sample along the way). External-caller detection strategies — analogous to
Wolverine.Http'sITenantDetectionPolicies(route/claim/subdomain-based) — are still open in #3368.Test plan
dotnet build wolverine.slnx -c Release— full solution, 0 errors/warningsdotnet test src/Wolverine.Grpc.Tests/Wolverine.Grpc.Tests.csproj— 255/255 passing, including 8 new tests:IMessageContext→ echoed back (proves the actual bug the docs/sample claimed was already fixed)PropagateEnvelopeHeaders = falseactually disables it, verified against a dedicated in-process hostAddWolverineGrpc()calls don't double-stack it) + interceptor ordering assertion (exception interceptor stays outermost)OrderChainWithGrpcsample separately does not actually run today (Dynamiccodegen mode throws — the sample projects don't referenceWolverine.RuntimeCompilation), confirming it was never actually executed/verified before — a pre-existing, unrelated issue left out of scope here. The in-process test suite exercises the identical code path (real KestrelTestServer, real interceptors, realBus.InvokeAsync).🤖 Generated with Claude Code